Asyncio Module Ported more tests [3/6]#744
Conversation
- Fixed a bug where an asyncio Lock was tried to be acquired without a release - Store asyncio tasks in order not to lose them - Removed start/shutdown from AsyncioReactor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #744 +/- ##
==========================================
+ Coverage 93.55% 94.04% +0.48%
==========================================
Files 389 389
Lines 24435 24435
==========================================
+ Hits 22861 22979 +118
+ Misses 1574 1456 -118 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
yuce
changed the title
yuce
changed the title
yuce
changed the title
|
|
||
| async def test_heartbeat_stopped_and_restored(self): | ||
| member2 = self.rc.startMember(self.cluster.id) | ||
| # TODO: remove this |
There was a problem hiding this comment.
| # asyncio tasks are weakly referenced | ||
| # storing tasks here in order not to lose them midway |
There was a problem hiding this comment.
I would add some brief note in the comment inline with the link you provided later (https://github.com/hazelcast/hazelcast-python-client/pull/744/files#r2585497868) as this comment is a little vague behind reasoning.
| def start(self): | ||
| self._is_live = True | ||
|
|
||
| def shutdown(self): | ||
| if not self._is_live: | ||
| return | ||
| # TODO: cancel tasks | ||
|
|
There was a problem hiding this comment.
minor: perhaps some comment on the reactor to tie its lifetime to that of the event loop ?
| ) | ||
|
|
||
| async def _start(self): | ||
| self._reactor.start() |
There was a problem hiding this comment.
Is this removed based on my comment w.r.t. the reactor lifetime?
There was a problem hiding this comment.
Yes, I mentioned that in a reply to your comment:
#741 (comment)
| invocation = Invocation(request, partition_id=1) | ||
|
|
||
| def mock(*_): | ||
| time.sleep(2) |
There was a problem hiding this comment.
As far as I see in the code that looks OK to me.
But I found another time.sleep in the tests and fixed it:
f1c8ce1
| await m.destroy() | ||
|
|
||
|
|
||
| def generate_key_owned_by_instance(client, uuid): |
There was a problem hiding this comment.
general: why don't we add explicit type specs in the new code? I know that it can be inferred with tooling but it seems like it's just a small bit of effort to make things readable, at least here for return type str
There was a problem hiding this comment.
I agree, especially for non-test code.
It's a bit too late for this PR, but I'll see what I can do for the new PRs.
5 participants
Ported the following tests from the asyncore client to the asyncio client: